Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add inrange2() to find points between two radiuses #52

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Add inrange2() to find points between two radiuses #52

wants to merge 4 commits into from

Conversation

chuong
Copy link

@chuong chuong commented Sep 4, 2017

This is useful when searching for points at a radius from fixed position with a tolerance. This is equivalent to, but more efficient than, using two inrange() operations for two different radiuses and then finding point that belong to only the larger radius.

@chuong
Copy link
Author

chuong commented Sep 4, 2017

Some tests in test_monkey.jl failed, but none of these relate to inrange2() implementation. Somehow I don't have these fail tests on my computer.

@KristofferC
Copy link
Owner

Thanks for the PR! Do you think perhaps in_range_interval is more descriptive? The 2 is a bit obscure to me, is it used in similar packages in other programming langauges?

Also, would be nice with a few more tests on a larger set of points where you compare that you get the same result for all the different type of trees.

@chuong
Copy link
Author

chuong commented Sep 15, 2017 •
edited
Loading

Thanks. Your suggestions are all possible. in_range_interval is a meaningful name. How about inranges?

I am struggling with naming the new function. On Wikipedia, new function is similar to range search, while the existing inrange function is called radiusSearch in Point Cloud Library and OpenCV. What do you think?

@KristofferC KristofferC mentioned this pull request Mar 16, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chuong @KristofferC